Add expense activity history foundation - #10237

$\textbf{🟡\ Medium\ Severity\ —\ Style}$

The changed permission-set source file is named ExpenseManagementObjects.permissionset.al, but the style contract requires <ObjectName>.<ObjectType>.al with the exact AL object type segment. Keeping permissionset lowercase weakens the file-system-to-object mapping that grep, tooling, and code navigation rely on; rename it to ExpenseManagementObjects.PermissionSet.al.

Knowledge:

• microsoft/knowledge/style/file-name-object-type-pattern.md

Line mapping was unavailable, so this was posted as an issue comment.

_{👍 useful · ❤️ especially valuable · 👎 wrong - reply with why · AL review agent v1.32.4}

The filename comment refers to a pre-existing repository file, ExpenseManagementObjects.permissionset.al, that this PR only updates. Renaming that shared permission-set file would be unrelated churn, so it is intentionally left unchanged.

$\textbf{🟠\ High\ Severity\ —\ Interfaces}$

Capability-specific behaviour is still selected with a case over the Expense Capability enum. In this PR, adding ActivityLog required synchronized edits to both the enum and IsEnabled, which is the variant-dispatch anti-pattern this rule calls out. Model the capability variants as enum-backed interface implementations and dispatch through an interface variable so future capabilities do not require editing the consumer codeunit.

Knowledge:

• microsoft/knowledge/interfaces/prefer-interface-over-case-branching.md

Line mapping was unavailable, so this was posted as an issue comment.

_{👍 useful · ❤️ especially valuable · 👎 wrong - reply with why · AL review agent v1.32.4}

$\textbf{🟠\ High\ Severity\ —\ Performance}$

ProcessExpenseReportLines calls CreateSalesDocument for every posted expense line, and CreateSalesDocument re-runs SalesHeader.FindFirst on the persistent Sales Header table each time to locate an open invoice for the same customer/currency/posting-date combination. That creates an N+1 lookup pattern during posting; cache or pre-group the target sales document (and its next line number) for the duration of the loop instead of querying Sales Header per line.

Knowledge:

• microsoft/knowledge/performance/avoid-get-inside-loop-on-large-table.md

Line mapping was unavailable, so this was posted as an issue comment.

_{👍 useful · ❤️ especially valuable · 👎 wrong - reply with why · AL review agent v1.32.4}

$\textbf{🟡\ Medium\ Severity\ —\ Security}$

The new audit logging trusts caller-supplied expense-user numbers for approval actions and records them as the actor without first binding them to the report's configured approver. In the internal Approve/Reject overloads, CheckApproverPermissions(ExpenseUser) only checks the global Can Approve flag on the supplied Expense User, unlike the session-based overload which additionally validates the caller against Expense Approval Setup/Default Approver No. for that specific report. An agent caller that can reach these internal actions can therefore attribute an approval or rejection to any user with Can Approve set, even one not configured as the approver for this report. Validate the supplied approver against the report's approval setup before state changes, and derive the logged actor from that validated approver rather than the raw request parameter.

_{Agent judgement — not directly backed by a BCQuality knowledge article.}

Line mapping was unavailable, so this was posted as an issue comment.

_{👍 useful · ❤️ especially valuable · 👎 wrong - reply with why · AL review agent v1.32.4}

$\textbf{🟡\ Medium\ Severity\ —\ Style}$

The changed file src/Apps/W1/ExpenseAgent/app/src/Permissions/ExpenseManagementObjects.permissionset.al uses a lowercase object-type suffix. The style rule for AL source files is <ObjectName>.<ObjectType>.al, so this permission set file should be renamed to ExpenseManagementObjects.PermissionSet.al to preserve the filesystem-to-object mapping that search and tooling rely on.

Knowledge:

• microsoft/knowledge/style/file-name-object-type-pattern.md

Line mapping was unavailable, so this was posted as an issue comment.

_{👍 useful · ❤️ especially valuable · 👎 wrong - reply with why · AL review agent v1.32.4}

$\textbf{🟠\ High\ Severity\ —\ Testing}$

ExpenseActivityLogAPITest.E2EActivityLogScenario uses a modal-page handler that always invokes OK without any enqueue/dequeue expectation, and the test does not prove which Expenses dialog was shown. That matches the hardcoded-handler anti-pattern: the scenario can stay green even if the wrong UI interaction occurs, so it does not reliably verify the headless UI path it claims to cover.

Knowledge:

• microsoft/knowledge/testing/ui-handlers-in-tests.md

Line mapping was unavailable, so this was posted as an issue comment.

_{👍 useful · ❤️ especially valuable · 👎 wrong - reply with why · AL review agent v1.32.4}

$\textbf{🟡\ Medium\ Severity\ —\ Style}$

This PR modifies src/Apps/W1/ExpenseAgent/app/src/Permissions/ExpenseManagementObjects.permissionset.al, but the file name does not follow the <ObjectName>.<ObjectType>.al convention. The object is a permissionset, so the type segment should be PermissionSet (ExpenseManagementObjects.PermissionSet.al), otherwise symbol-to-file lookup and tooling stay inconsistent for a file this PR is already touching.

Knowledge:

• microsoft/knowledge/style/file-name-object-type-pattern.md

Line mapping was unavailable, so this was posted as an issue comment.

_{👍 useful · ❤️ especially valuable · 👎 wrong - reply with why · AL review agent v1.32.4}

$\textbf{🟡\ Medium\ Severity\ —\ Error\ Handling}$

if TrySendReimbursementNotification(PostedExpenseReportHeader) then; technically enables try semantics, but the empty branch discards false and never handles the captured error. That silently swallows reimbursement-notification failures during posting; consume the Boolean result in a real branch and handle the failure immediately while the last-error state is still available.

Knowledge:

• microsoft/knowledge/error-handling/ignored-tryfunction-return-disables-try-semantics.md

Line mapping was unavailable, so this was posted as an issue comment.

_{👍 useful · ❤️ especially valuable · 👎 wrong - reply with why · AL review agent v1.32.4}

$\textbf{🟡\ Medium\ Severity\ —\ Accessibility}$

This promoted navigation group is captioned 'Expense', but the actions inside operate on the current expense report. Using the entity name 'Expense Report' matches the standard promoted-group naming guidance and aligns this page with the posted expense report page added in the same change.

Suggested fix (apply manually — could not be anchored as a one-click suggestion):

                Caption = 'Expense Report';

Knowledge:

• microsoft/knowledge/ui/promoted-action-groups.md

Line mapping was unavailable, so this was posted as an issue comment.

_{👍 useful · ❤️ especially valuable · 👎 wrong - reply with why · AL review agent v1.32.4}

$\textbf{🟡\ Medium\ Severity\ —\ Accessibility}$

The promoted action group uses the nonstandard caption 'Approval'. BC's standard workflow group name is 'Approve', so keeping 'Approval' here makes this action bar diverge from the platform's established document-page vocabulary.

Suggested fix (apply manually — could not be anchored as a one-click suggestion):

                Caption = 'Approve';

Knowledge:

• microsoft/knowledge/ui/promoted-action-groups.md

Line mapping was unavailable, so this was posted as an issue comment.

_{👍 useful · ❤️ especially valuable · 👎 wrong - reply with why · AL review agent v1.32.4}

$\textbf{🟡\ Medium\ Severity\ —\ Accessibility}$

This promoted navigation group is captioned 'Expense', but the actions inside are tied to the current expense report record. The standard pattern is to use an entity-named group, so captioning it 'Expense Report' keeps the ribbon consistent with native BC pages.

Suggested fix (apply manually — could not be anchored as a one-click suggestion):

                Caption = 'Expense Report';

Knowledge:

• microsoft/knowledge/ui/promoted-action-groups.md

Line mapping was unavailable, so this was posted as an issue comment.

_{👍 useful · ❤️ especially valuable · 👎 wrong - reply with why · AL review agent v1.32.4}

$\textbf{🟠\ High\ Severity\ —\ Privacy}$

The new reimbursement-notification flow sends the employee email in the "On-Behalf-Of" header and the posted expense report id to the external Expense Agent service, but this path never checks the Expense Agent privacy notice before calling SendReimbursementNotification. The integration should confirm or verify its own notice on this request path before transferring data externally.

Knowledge:

• microsoft/knowledge/privacy/privacy-notice-consent-for-external-data-transfer.md

Line mapping was unavailable, so this was posted as an issue comment.

_{👍 useful · ❤️ especially valuable · 👎 wrong - reply with why · AL review agent v1.32.4}

$\textbf{🟠\ High\ Severity\ —\ Privacy}$

The new Expense Report Header table relies on table-level DataClassification and leaves 25 stored fields without explicit field classifications, including "Expense User No.", "Expense User Name", "Description", and "Spend Request No.". Table-level classification does not satisfy field-level privacy classification requirements, so these personal and customer-bearing fields ship under-classified.

Knowledge:

• microsoft/knowledge/privacy/table-level-data-classification-cascades.md
• microsoft/knowledge/privacy/data-classification-required-on-pii-fields.md

Line mapping was unavailable, so this was posted as an issue comment.

_{👍 useful · ❤️ especially valuable · 👎 wrong - reply with why · AL review agent v1.32.4}

$\textbf{🟠\ High\ Severity\ —\ Privacy}$

The new Posted Expense Report Header table also relies on table-level DataClassification and leaves 25 stored fields without explicit field classifications, including "Expense User No.", "Expense User Name", "Description", and "Spend Request No.". Field-level classifications are still required for these persisted values.

Knowledge:

• microsoft/knowledge/privacy/table-level-data-classification-cascades.md
• microsoft/knowledge/privacy/data-classification-required-on-pii-fields.md

Line mapping was unavailable, so this was posted as an issue comment.

_{👍 useful · ❤️ especially valuable · 👎 wrong - reply with why · AL review agent v1.32.4}

Superseded by a replacement PR from the same branch to reset stale review and CI history after the design was finalized.

$\textbf{🟠\ High\ Severity\ —\ Interfaces}$

This change adds another branch to the existing case Capability of dispatcher in "Expense Capabilities Provider".IsEnabled. Adding the new ActivityLog capability now requires editing the consumer again instead of plugging in a new implementation behind an interface, which is the enum-dispatch anti-pattern the interfaces guidance warns against.

Knowledge:

• microsoft/knowledge/interfaces/prefer-interface-over-case-branching.md

Line mapping was unavailable, so this was posted as an issue comment.

_{👍 useful · ❤️ especially valuable · 👎 wrong - reply with why · AL review agent v1.32.4}

$\textbf{🟠\ High\ Severity\ —\ Privacy}$

The new reimbursement-notification path sends data to the external Expense Agent service without checking the custom privacy notice on the sending path. Both TrySendReimbursementNotification() and CheckAndSendReimbursementNotification() proceed from configuration checks straight to EAHttpClient.SendReimbursementNotification(ExpenseUser2."E-mail", PostedExpReportHeader.SystemId), so the expense user's email address and report identifier can be transferred even though this code never calls ConfirmPrivacyNoticeApproval(...) or GetPrivacyNoticeApprovalState(...) before the request.

Knowledge:

• microsoft/knowledge/privacy/privacy-notice-consent-for-external-data-transfer.md

Line mapping was unavailable, so this was posted as an issue comment.

_{👍 useful · ❤️ especially valuable · 👎 wrong - reply with why · AL review agent v1.32.4}

$\textbf{🟡\ Medium\ Severity\ —\ Style}$

UpdateReportLines builds a user-facing confirmation from CanModifyLinesQst, but the routine is driven by Rec.FieldName(...) values. That leaks developer identifiers into the prompt instead of translated captions, even though the label comment says %1 = Field Caption. Use a user-facing FieldCaption(...) value for the message and keep any internal selector separate.

Knowledge:

• microsoft/knowledge/style/fieldcaption-not-fieldname-in-user-messages.md

Line mapping was unavailable, so this was posted as an issue comment.

_{👍 useful · ❤️ especially valuable · 👎 wrong - reply with why · AL review agent v1.32.4}

$\textbf{🟡\ Medium\ Severity\ —\ Style}$

EntryRecIDLbl contains two placeholders (%1 %2) but has no Comment, so translators cannot tell that ShowDimensions() passes the table caption and document number. Add a placeholder comment at the declaration.

Suggested fix (apply manually — could not be anchored as a one-click suggestion):

        EntryRecIDLbl: Label '%1 %2', Comment = '%1 = Table Caption, %2 = Document No.', Locked = true;

Knowledge:

• microsoft/knowledge/style/label-comment-explains-placeholders.md

Line mapping was unavailable, so this was posted as an issue comment.

_{👍 useful · ❤️ especially valuable · 👎 wrong - reply with why · AL review agent v1.32.4}

$\textbf{🟠\ High\ Severity\ —\ Web\ Services}$

This page is keyed by SystemId, but the expenseReportLines navigation property links children by "Document No." = field("No.") instead of the report's SystemId. That makes the navigation property depend on a business key rather than the API contract's stable GUID. Add a parent-SystemId foreign key to the line API/table and link the part with field(SystemId).

Knowledge:

• microsoft/knowledge/web-services/link-api-parts-on-systemid-and-set-multiplicity.md

Line mapping was unavailable, so this was posted as an issue comment.

_{👍 useful · ❤️ especially valuable · 👎 wrong - reply with why · AL review agent v1.32.4}

$\textbf{🟠\ High\ Severity\ —\ Web\ Services}$

This page is keyed by SystemId, but the expenses, expenseReports, approverView, and expenseApprovalSetup navigation properties still link child APIs on field("No."). That creates a second identity scheme for navigation, so those child links depend on a renamable business key instead of the API contract's stable GUID. Add SystemId-based foreign keys on the child APIs and switch these SubPageLink clauses to field(SystemId).

Knowledge:

• microsoft/knowledge/web-services/link-api-parts-on-systemid-and-set-multiplicity.md

Line mapping was unavailable, so this was posted as an issue comment.

_{👍 useful · ❤️ especially valuable · 👎 wrong - reply with why · AL review agent v1.32.4}

$\textbf{🟠\ High\ Severity\ —\ Web\ Services}$

This page is keyed by SystemId, but the postedExpenseReportLines navigation property links children by "Document No." = field("No.") instead of the posted report's SystemId. That makes the navigation property depend on a business key rather than the API contract's stable GUID. Add a parent-SystemId foreign key to the posted line API/table and link the part with field(SystemId).

Knowledge:

• microsoft/knowledge/web-services/link-api-parts-on-systemid-and-set-multiplicity.md

Line mapping was unavailable, so this was posted as an issue comment.

_{👍 useful · ❤️ especially valuable · 👎 wrong - reply with why · AL review agent v1.32.4}